feat(cloudflare): Instrument sync KV#21316
Open
JPeer264 wants to merge 1 commit into
Open
Conversation
JPeer264
requested review from
andreiborza and
mydea
and removed request for
a team
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2decb59. Configure here.
JPeer264
force-pushed
the
jp/cloudflare-sync-kv-instrument
branch
from
2decb59 to
091b09b
Compare
andreiborza
approved these changes
Jun 3, 2026
JPeer264
force-pushed
the
jp/cloudflare-sync-kv-instrument
branch
from
0526d91 to
26e3446
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+39
to
+41
| if (prop === 'kv' && original != null && typeof original === 'object') { | ||
| return instrumentDurableObjectSyncKvStorage(original as SyncKvStorage); | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: Accessing storage.kv repeatedly creates a new proxy object each time instead of reusing a cached one, causing unnecessary performance overhead.
Severity: MEDIUM
Suggested Fix
Use a WeakMap to cache the instrumented proxy for the kv property. On the first access, create the proxy, store it in the map, and return it. On subsequent accesses, return the cached proxy from the WeakMap. This aligns with the existing caching pattern in instrumentEnv.ts and avoids creating multiple proxy objects.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts#L39-L41
Potential issue: The instrumentation for Durable Object storage creates a new proxy for
the `kv` property on every access. In scenarios where code makes multiple sequential
calls like `storage.kv.get(...)` and then `storage.kv.put(...)`, a new proxy object is
unnecessarily created for each call. This leads to increased object allocations and
garbage collection pressure, which can degrade performance in serverless environments.
This behavior is inconsistent with the caching pattern (using `WeakMap`) established in
other parts of the codebase, such as in `instrumentEnv.ts`.
Did we get this right? 👍 / 👎 to inform future reviews.
Contributor
size-limit report 📦
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #20830
closes JS-2443
This adds instrumentation for
ctx.storage.kv.*(it only adds a span like with the async KV).Cloudflare itself is having
cloudflare-durable-object-sqlasdb.system.name, that is why we have it here now too.